Skip to content

feat: add dynamic parameter type extensions#4290

Open
calebdw wants to merge 4 commits intophpstan:2.1.xfrom
calebdw:dynamic_parameter_extension
Open

feat: add dynamic parameter type extensions#4290
calebdw wants to merge 4 commits intophpstan:2.1.xfrom
calebdw:dynamic_parameter_extension

Conversation

@calebdw
Copy link
Contributor

@calebdw calebdw commented Sep 5, 2025 •
edited
Loading

Closes phpstan/phpstan#11707, closes phpstan/phpstan#12585

Supersedes #3828, supersedes #3131, supersedes #3823

Hello!

This adds generalized dynamic parameter type extensions and deprecates the parameter closure type extensions per phpstan/phpstan#11707 (comment).

This also fixes the return.type and the argument.type errors described in phpstan/phpstan#12585 when the parameter type is overridden via the extension.

Note

The original type from the closure type extensions was being passed around as $passedToType to the methods that needed it. However, I opted to rename this to overriddenType since I wasn't quite sure what $passedToType really meant. Let me know if there's something you'd rather do differently.

CC: @canvural, @Neol3108

Thanks!

@calebdw calebdw force-pushed the dynamic_parameter_extension branch 4 times, most recently from 49c7da8 to 58e51fb Compare September 5, 2025 14:15
@calebdw calebdw marked this pull request as draft September 5, 2025 16:35
@calebdw calebdw force-pushed the dynamic_parameter_extension branch from 58e51fb to 79de0a6 Compare September 5, 2025 17:05
@calebdw calebdw marked this pull request as ready for review September 5, 2025 17:36
@phpstan-bot
Copy link
Collaborator

phpstan-bot commented Sep 5, 2025

This pull request has been marked as ready for review.

@canvural
Copy link
Contributor

canvural commented Sep 6, 2025

I tested it by porting my previous implementation for Larastan to this one, and looks like it works fine! Will try to implement more use cases and see if I can find any bugs. Also will try to test it on real projects.But so far so good. Thanks for this!

I'll also try to review this PR (though Ondrej would do a better job 😄 )

@canvural canvural mentioned this pull request Sep 7, 2025
Draft
2 tasks
@calebdw calebdw force-pushed the dynamic_parameter_extension branch from 79de0a6 to 7c605c1 Compare September 13, 2025 12:52
ondrejmirtes
ondrejmirtes requested changes Sep 14, 2025
View reviewed changes
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks solid, just a few questions as a start, will do deep review later 👍

@canvural
Copy link
Contributor

canvural commented Oct 2, 2025

@calebdw @ondrejmirtes Sorry to bother you, but I wanted to ask how we can move this forward? I'm really interested in this feature.

@calebdw
Copy link
Contributor Author

calebdw commented Oct 2, 2025

No worries, just been busy with personal life, I'll try to get to the comments soon

@calebdw calebdw force-pushed the dynamic_parameter_extension branch 2 times, most recently from b9ce6ac to b293d32 Compare November 10, 2025 15:46
@calebdw
Copy link
Contributor Author

calebdw commented Nov 10, 2025

@ondrejmirtes, @canvural, sorry it took so long to address these comments---just been busy with personal life but hoping to move this forward now 🙏

@calebdw calebdw requested a review from ondrejmirtes November 10, 2025 15:46
@calebdw calebdw mentioned this pull request Nov 26, 2025
Open
@calebdw calebdw force-pushed the dynamic_parameter_extension branch from b293d32 to 16430bf Compare November 26, 2025 14:33
@CamKemBell
Copy link

CamKemBell commented Jan 9, 2026

@calebdw - is this still planned on going ahead? Does this fix the issue with using whereHas with custom builders?

I've seen multiple projects now look to using custom builders since the implementation of the attribute & official support from Laravel last year, only to find issues when trying to type the arguments in eloquent method for type hints / auto complete, etc.

https://laravel-news.com/defining-a-dedicated-query-builder-in-laravel-12-with-php-attributes

In the meantime, we have been using whereRelation, which compiles to whereHas under the hood, but it would be nice to use whereHas directly.

image

@dnyg
Copy link

dnyg commented Jan 15, 2026

My team are also eagerly awaiting the outcome of this PR! Hope it can be pushed forward 🙏

@calebdw calebdw force-pushed the dynamic_parameter_extension branch 2 times, most recently from 08bc956 to 7823716 Compare January 19, 2026 15:17
@calebdw
Copy link
Contributor Author

calebdw commented Jan 19, 2026

@CamKemBell, yes I just rebased and resolved all the conflicts

@ondrejmirtes, this is ready to review 👍

@calebdw calebdw force-pushed the dynamic_parameter_extension branch from 7823716 to 9e68ce4 Compare January 23, 2026 22:17
@ruudk ruudk mentioned this pull request Jan 25, 2026
Merged
@dnyg
Copy link

dnyg commented Feb 12, 2026

What is the status on this PR?

@VincentLanglet
Copy link
Contributor

VincentLanglet commented Feb 14, 2026

Hi @calebdw sorry for the delay there is now more review for the PHPStan codebase.

Are you still interested by the PR ?
There is a lot of conflict, can you solve them please ?

Thanks !

@calebdw calebdw force-pushed the dynamic_parameter_extension branch from 9e68ce4 to c67e140 Compare February 16, 2026 14:43
@calebdw
Copy link
Contributor Author

calebdw commented Feb 16, 2026

@VincentLanglet, yes, the conflicts have been resolved!

@calebdw calebdw force-pushed the dynamic_parameter_extension branch 4 times, most recently from 40add44 to 7d80107 Compare February 16, 2026 15:29
VincentLanglet
VincentLanglet reviewed Feb 16, 2026
View reviewed changes
@calebdw calebdw force-pushed the dynamic_parameter_extension branch 3 times, most recently from 1a52f81 to 5dd28d7 Compare February 18, 2026 09:19
@VincentLanglet
Copy link
Contributor

VincentLanglet commented Feb 18, 2026 •
edited
Loading

Not sure if all the failure are related to the pr ; but there is more failure than on other PR.
You should try fix level test + test with php 7.4 (and maybe reduce the number of escaped mutant)

@calebdw calebdw force-pushed the dynamic_parameter_extension branch 2 times, most recently from a699133 to c89c885 Compare February 18, 2026 12:33
@calebdw calebdw force-pushed the dynamic_parameter_extension branch from c89c885 to 781f1dc Compare February 18, 2026 14:14
@calebdw calebdw force-pushed the dynamic_parameter_extension branch from 781f1dc to f59eab2 Compare February 18, 2026 14:37
@calebdw
Copy link
Contributor Author

calebdw commented Feb 18, 2026

@VincentLanglet,

  • I've fixed the PHP 7.4 tests
  • most of the integration tests are failing due to the deprecation warning from the old interfaces---not sure there's any way to fix these besides the respective projects updating their baselines after this has been released
  • the Level test failures appear unrelated
  • I'm not too sure how to prevent the escape mutants, some of these would already be skipped (e.g., if an array is not a constant array) so having a test case for every mutant is tricky

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VincentLanglet VincentLanglet VincentLanglet left review comments

@ondrejmirtes ondrejmirtes Awaiting requested review from ondrejmirtes

@staabm staabm Awaiting requested review from staabm

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Invalid argument types when using MethodParameterClosureTypeExtension Allow specifying that parameters should be covariant

7 participants

@calebdw @phpstan-bot @canvural @CamKemBell @dnyg @VincentLanglet @ondrejmirtes

Comments